[operations][client-preset] CODEGEN-500 - Support semantic non-null for clients#10323
[operations][client-preset] CODEGEN-500 - Support semantic non-null for clients#10323
Conversation
🦋 Changeset detectedLatest commit: 739ada6 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
| semanticNonNull?: { | ||
| errorHandlingClient: boolean; | ||
| }; |
There was a problem hiding this comment.
I'm thinking this object can be extended to handle other related semanticNonNull functionalities e.g. @catch?
(I'm not entirely sure how it'd work yet, but I imagine there could be options we turn on/off here 😅 )
There was a problem hiding this comment.
I see @catch as mainly orthogonal to @semanticNonNull. Sure they make a lot of sense together but you can have an error handling client that uses @catch(to: RESULT) or @catch(to: THROW) without semantic nullability on the server.
That client will not get the semantic nullability information but could still use @catch to modify the shape of the generated code.
I would keep it simple/yagni with just a plain Boolean option:
semanticNonNull?: booleanIf really you want to bundle nullability options together, maybe something like so:
nullability?: {
semanticNonNull: boolean;
catch: boolean;
}
🚀 Snapshot Release (
|
| Package | Version | Info |
|---|---|---|
@graphql-codegen/visitor-plugin-common |
5.8.0-alpha-20250326083144-739ada6d1bd51f5f1a505e18b205b099c796aad3 |
npm ↗︎ unpkg ↗︎ |
@graphql-codegen/typescript-document-nodes |
4.0.16-alpha-20250326083144-739ada6d1bd51f5f1a505e18b205b099c796aad3 |
npm ↗︎ unpkg ↗︎ |
@graphql-codegen/gql-tag-operations |
4.0.17-alpha-20250326083144-739ada6d1bd51f5f1a505e18b205b099c796aad3 |
npm ↗︎ unpkg ↗︎ |
@graphql-codegen/typescript-operations |
4.6.0-alpha-20250326083144-739ada6d1bd51f5f1a505e18b205b099c796aad3 |
npm ↗︎ unpkg ↗︎ |
@graphql-codegen/typescript-resolvers |
4.5.0-alpha-20250326083144-739ada6d1bd51f5f1a505e18b205b099c796aad3 |
npm ↗︎ unpkg ↗︎ |
@graphql-codegen/typed-document-node |
5.1.1-alpha-20250326083144-739ada6d1bd51f5f1a505e18b205b099c796aad3 |
npm ↗︎ unpkg ↗︎ |
@graphql-codegen/typescript |
4.1.6-alpha-20250326083144-739ada6d1bd51f5f1a505e18b205b099c796aad3 |
npm ↗︎ unpkg ↗︎ |
@graphql-codegen/client-preset |
4.8.0-alpha-20250326083144-739ada6d1bd51f5f1a505e18b205b099c796aad3 |
npm ↗︎ unpkg ↗︎ |
@graphql-codegen/graphql-modules-preset |
4.0.16-alpha-20250326083144-739ada6d1bd51f5f1a505e18b205b099c796aad3 |
npm ↗︎ unpkg ↗︎ |
| const isPersistedOperations = !!options.presetConfig?.persistedDocuments; | ||
| if (options.config.semanticNonNull?.errorHandlingClient) { | ||
| options.schemaAst = await semanticToStrict(options.schemaAst!); | ||
| options.schema = options.schemaAst as any; // FIXME: `schemaAst` _seems_ to be able to used as `schema`, but not sure if it has any side effects? |
There was a problem hiding this comment.
I'm a bit unsure about this:
schemais aDocumentNodeshemaAstis aGraphQLSchema- We can forcefully override
schemawithschemaAst... and it works 🤔
Maybe when loading schema in each generates block, we do a check? That's my best guess though, and if anyone has more context, please let me know!
EDIT: Looks like it's somewhere here 👀
There was a problem hiding this comment.
Maybe what you want here is this?
| options.schema = options.schemaAst as any; // FIXME: `schemaAst` _seems_ to be able to used as `schema`, but not sure if it has any side effects? | |
| options.schema = parse(printSchema(options.schemaAst)); |
Just guessing from the types you mention, I'm not familiar with this codebase.
💻 Website PreviewThe latest changes are available as preview in: https://pr-10323.graphql-code-generator.pages.dev |
|
Hi @benjie @martinbonnin, this PR is the client type implementation |
benjie
left a comment
There was a problem hiding this comment.
LGTM! I've left a few suggestions, but really excited to see this get merged! 🚀
| * export default config; | ||
| * ``` | ||
| */ | ||
| semanticNonNull?: { |
There was a problem hiding this comment.
Perhaps, and this is just a suggestion, rename this to "semantic nullability" as a more generic term. If we were to accept a solution that used a document directive to enable the syntax, this is the likely name that we would use for that directive, and it doesn't tie the solution to one specific implementation. (GraphQL-SOCK can be updated to work with other solutions without any effort on your part.)
| semanticNonNull?: { | |
| semanticNullability?: { |
| schema: GraphQLSchema, | ||
| rawDocuments: Types.DocumentFile[], | ||
| config: TypeScriptDocumentsPluginConfig | ||
| ) => { | ||
| const transformedSchema = config.semanticNonNull?.errorHandlingClient ? await semanticToStrict(schema) : schema; |
There was a problem hiding this comment.
I personally would rename the schema variable to inputSchema so that schema, the variable you'd use by default, is the correct variable to use:
| schema: GraphQLSchema, | |
| rawDocuments: Types.DocumentFile[], | |
| config: TypeScriptDocumentsPluginConfig | |
| ) => { | |
| const transformedSchema = config.semanticNonNull?.errorHandlingClient ? await semanticToStrict(schema) : schema; | |
| inputSchema: GraphQLSchema, | |
| rawDocuments: Types.DocumentFile[], | |
| config: TypeScriptDocumentsPluginConfig | |
| ) => { | |
| const schema = config.semanticNonNull?.errorHandlingClient ? await semanticToStrict(inputSchema) : inputSchema; |
(This would also mean no other changes were needed in the function.)
| return sock.semanticToStrict(schema); | ||
| } catch { | ||
| throw new Error( | ||
| "To use the `customDirective.semanticNonNull` option, you must install the 'graphql-sock' package." |
There was a problem hiding this comment.
| "To use the `customDirective.semanticNonNull` option, you must install the 'graphql-sock' package." | |
| "To use the `semanticNonNull.errorHandlingClient` option, you must install the 'graphql-sock' package." |
| const isPersistedOperations = !!options.presetConfig?.persistedDocuments; | ||
| if (options.config.semanticNonNull?.errorHandlingClient) { | ||
| options.schemaAst = await semanticToStrict(options.schemaAst!); | ||
| options.schema = options.schemaAst as any; // FIXME: `schemaAst` _seems_ to be able to used as `schema`, but not sure if it has any side effects? |
There was a problem hiding this comment.
Maybe what you want here is this?
| options.schema = options.schemaAst as any; // FIXME: `schemaAst` _seems_ to be able to used as `schema`, but not sure if it has any side effects? | |
| options.schema = parse(printSchema(options.schemaAst)); |
Just guessing from the types you mention, I'm not familiar with this codebase.
| - [`nonOptionalTypename`](/plugins/typescript/typescript#nonoptionaltypename): Automatically adds `__typename` field to the generated types, even when they are not specified in the selection set, and makes it non-optional. | ||
| - [`avoidOptionals`](/plugins/typescript/typescript#avoidoptionals): This will cause the generator to avoid using TypeScript optionals (`?`) on types. | ||
| - [`documentMode`](#documentmode): Allows you to control how the documents are generated. | ||
| - [`semanticNonNull`](/plugins/typescript/typescript-operations#semanticnonnull): Handles `@semanticNonNull` directive based on client-side requirements. |
There was a problem hiding this comment.
| - [`semanticNonNull`](/plugins/typescript/typescript-operations#semanticnonnull): Handles `@semanticNonNull` directive based on client-side requirements. | |
| - [`semanticNullability`](/plugins/typescript/typescript-operations#semanticnullability): Indicate the client capabilities to get stronger types with [semantic nullability](https://github.com/graphql/graphql-wg/blob/main/rfcs/SemanticNullability.md)-enabled schemas. |
There was a problem hiding this comment.
I like semanticNonNull. This option is part of the overall solution but doesn't solve all of semantic nullability. It's very specific about the codegen and the support of @semanticNonNull but doesn't touch the runtime.
Counter proposal:
| - [`semanticNonNull`](/plugins/typescript/typescript-operations#semanticnonnull): Handles `@semanticNonNull` directive based on client-side requirements. | |
| - [`semanticNonNull`](/plugins/typescript/typescript-operations#semanticnonnull): fields marked as semantically non null using the [`@semanticNonNull` directive](https://specs.apollo.dev/nullability/v0.2/#@semanticNonNull) | |
| are generated as non-null in TypeScript. | |
| Enabling this config requires an error-handling client. This can for an example be done using [graphql-toe](https://github.com/graphile/graphql-toe). |
There was a problem hiding this comment.
By using graphql-sock this supports not only @semanticNonNull the directive but all the proposed solutions (assuming GraphQL.js support) to the semantic nullability RFC. Theoretically this should already work just fine with the * syntax when using graphql@canary-pr-4192 for example (I've not tested it though). If we were to come up with an alternative solution (e.g. GraphQLStrictNonNull to represent the old "kills parent on exception" style of non-null) then "semantic nullability" would still cover this solution, but "semanticNonNull" would not.
There was a problem hiding this comment.
I'd support renaming the option to simply nullability though, as you proposed separately.
martinbonnin
left a comment
There was a problem hiding this comment.
Looks great!! A couple documentation/terminology comments but otherwise looking forward to play with this 🚀 !!
| semanticNonNull?: { | ||
| errorHandlingClient: boolean; | ||
| }; |
There was a problem hiding this comment.
I see @catch as mainly orthogonal to @semanticNonNull. Sure they make a lot of sense together but you can have an error handling client that uses @catch(to: RESULT) or @catch(to: THROW) without semantic nullability on the server.
That client will not get the semantic nullability information but could still use @catch to modify the shape of the generated code.
I would keep it simple/yagni with just a plain Boolean option:
semanticNonNull?: booleanIf really you want to bundle nullability options together, maybe something like so:
nullability?: {
semanticNonNull: boolean;
catch: boolean;
}| - [`nonOptionalTypename`](/plugins/typescript/typescript#nonoptionaltypename): Automatically adds `__typename` field to the generated types, even when they are not specified in the selection set, and makes it non-optional. | ||
| - [`avoidOptionals`](/plugins/typescript/typescript#avoidoptionals): This will cause the generator to avoid using TypeScript optionals (`?`) on types. | ||
| - [`documentMode`](#documentmode): Allows you to control how the documents are generated. | ||
| - [`semanticNonNull`](/plugins/typescript/typescript-operations#semanticnonnull): Handles `@semanticNonNull` directive based on client-side requirements. |
There was a problem hiding this comment.
I like semanticNonNull. This option is part of the overall solution but doesn't solve all of semantic nullability. It's very specific about the codegen and the support of @semanticNonNull but doesn't touch the runtime.
Counter proposal:
| - [`semanticNonNull`](/plugins/typescript/typescript-operations#semanticnonnull): Handles `@semanticNonNull` directive based on client-side requirements. | |
| - [`semanticNonNull`](/plugins/typescript/typescript-operations#semanticnonnull): fields marked as semantically non null using the [`@semanticNonNull` directive](https://specs.apollo.dev/nullability/v0.2/#@semanticNonNull) | |
| are generated as non-null in TypeScript. | |
| Enabling this config requires an error-handling client. This can for an example be done using [graphql-toe](https://github.com/graphile/graphql-toe). |
|
Thanks @benjie @martinbonnin for your great suggestions! I've manually applied the suggestions because the changes need to happen across multiple files:
|
|
Merging this so we can try out the alpha versions! We can make changes before the final release if needed 🙂 |
See the awesome work from @eddeee888 in dotansimha/graphql-code-generator#10323
Description
This PR supports
@semanticNonNullsupport for client use cases:typescript-operationsclient-presetIntroducing
semanticNonNullconfig option. This will be the target option for all semantic non null options in the future.For now, it includes an option called
errorHandlingClientso users can declare whether they are using an error-handling client or not:Type of change
How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration